This repository has been archived by the owner on Mar 26, 2024. It is now read-only.
Filter Reports (index page) by status #112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
self-assigned this
|
This looks great. Thanks. Is there any way I can get you to write some rspec tests to ensure the tabs behave how they're supposed to? |
|
Sure, I'll try to get to it early next week. |
|
I get the following error when trying to run Are you familiar with this at all? |
|
I went ahead and committed some rspec's despite the fact that I can't figure out how to get spec to run. Perhaps someone would be willing to help me get rspec working (probably a gem version issue) or just checkout my spec's and let me know if they run successfully. |
|
Tests verified. Run the following to verify: Note that the master branch currently contains 3 broken tests in this spec file. They are unrelated to my changes. All new tests related to my changes pass. Sorry that I didn't have time to research and fix the broken ones. |
|
I get these errors in the rails3 branch, which are the only ones I haven't figured out. Are they the same as you see on master (I've never gotten the tests running on master at all) -- they're in models/report, but I wonder if they underlie the controller errors. |
|
Doesn't look like the same ones. Here are the ones I get from master branch. I openly admit that these failures may also be related to an issue with my gem versions. Just curious, is bundler being used in the Rails 3 branch? |
|
Oh, gotcha. In |
|
I made the change you requested and still get the same errors. |
|
I just found on Mocha's website: |
|
I believe that this is ready to merge. The tests related to my code is passing. There are some failures, but they're not related to the code I wrote. Notes: I'm using mocha v0.12.3 and rails v3.2.8 |
|
@gposton Could you rebase your branch to current master, and squash a single commit? |
|
First time I've done a rebase as opposed to merging. I love git more and more everyday. Rebase and squash complete. |
|
Nice! I think the only change to complain about is mocha vs. mocha_standalone in spec/spec_helpers. I'll try to test this change out in the next few days, but from a code perspective I like it. Not required, but to help showcase the change, could you post 1-2 cropped screenshots of the new tabs and screens? |
|
|
|
Totally awesome. I'm all for this. @puppetlabs opinion? |
|
@gposton please change the spec/spec_helper mocha_standalone back to plain mocha -- for version 0.9.7 that's bundled locally in the PD repo plain 'mocha' appears to be preferred. Squash the change in and I'll merge asap. |
- supporting named_scopes in report model - supporting routes - supporting controller actions
|
@sodabrew mocha_standalone has been reverted back to mocha as requested |
sodabrew
added a commit
that referenced
this pull request
Dec 19, 2012
Filter Reports (index page) by status
|
@sodabrew @gposton I've put a pull in to revert this. As mentioned in that pull, a migration uses the reports model to make the migration, so when the report model changes, that migration breaks. There are two basic solutions to this that I see: a.) update the old migration to not use the model b.) update the report changes to use a second table that is then joined against the reports Without a, the reports model can never change, so it is probably the better of the two solutions, even though I hate the idea of editing old migrations. |
adaburrows
added a commit
that referenced
this pull request
Dec 19, 2012
Revert "Merge pull request #112 from gposton/master"
|
@gposton Please resubmit with this scope removed -- overriding Report.all is apparently bad news. named_scope :all, :conditions => {:kind => "apply"}, :include => :metrics |
|
For posterity, the Gem errors above were caused by a too-new version of Rubygems. Turns out that Dashboard requires version 1.3.x, begins warning on higher versions, and begins failing somewhere around version 1.8.x. |
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request enables filtering of reports by the report's status.
The UI is modeled after the similar functionality for filtering nodes by 'last run' status (tabs for 'changed', 'changed', 'unchanged', 'failed', 'etc').
Value:
I've often found myself wanting the ability to quickly find the last updates applied to the environment. With our environment (that is fairly static most of the time), I spent quite a bit of time paging through unchanged reports before finding the last 'changed' report.